-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Rust: Update legacy MaD models 3 #19946
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There's currently a large number of test changes caused by this PR (highlighted by CI). I think it would be prudent to get the first two PRs merged before attempting to address this. |
- ["repo:https://github.com/hyperium/hyper-util:hyper-util", "<crate::client::legacy::Client>::get", "ReturnValue.Future", "remote", "manual"] | ||
- ["repo:https://github.com/hyperium/hyper-util:hyper-util", "<crate::client::legacy::Client>::request", "ReturnValue.Future", "remote", "manual"] | ||
- ["<hyper::client::conn::http1::SendRequest>::send_request", "ReturnValue.Future.Field[core::result::Result::Ok(0)]", "remote", "manual"] | ||
- ["<hyper::client::conn::http2::SendRequest>::send_request", "ReturnValue.Future.Field[core::result::Result::Ok(0)]", "remote", "manual"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test runs show failures for sender.send_request()
not being recognised as remote source. The model looks fine though. I expect the problem is a failure in the type inferencer. I think the following type annotation should make the test pass again:
- let (mut sender, conn) = hyper::client::conn::http1::handshake(io).await?;
+ let (sender, conn) = hyper::client::conn::http1::handshake(io).await?;
+ let mut sender: hyper::client::conn::http1::SendRequest<String> = sender;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, it works better like that. I've pushed some updates, we'll see how the other tests look now.
Created #20004 to log the type inference issue here.
Update even more legacy MaD models to the new model format (continues from #19942 and should be independent of that).